Skip to content

fix(agentex-ui): abort the upstream request when the BFF client disconnects - #383

Open
erichwoo-scale wants to merge 2 commits into
mainfrom
erichwoo/bff-abort-upstream-on-disconnect
Open

fix(agentex-ui): abort the upstream request when the BFF client disconnects#383
erichwoo-scale wants to merge 2 commits into
mainfrom
erichwoo/bff-abort-upstream-on-disconnect

Conversation

@erichwoo-scale

@erichwoo-scale erichwoo-scale commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Problem

The /api/agentex catch-all BFF proxy streams SSE through unbuffered but passes no signal to fetch. When the browser goes away — closed tab, navigation, or the 300s Istio route timeout on the UI's virtualService — undici keeps the upstream request open. There is no timeout on the call either, so an orphan never expires, and the browser's EventSource then reconnects and opens a fresh one.

Each orphan leaves an agentex handler blocked in a Redis XREAD, pinning one connection from that pod's redis-py pool (REDIS_MAX_CONNECTIONS, default 200). They accumulate until the pools are exhausted, at which point agentex fails to serve new requests and only a restart recovers it.

Reported on Mayo Nonprod (Slack thread). At saturation Redis showed 1,094 connected_clients with 943 blocked_clients, while each of the two agentex-ui pods held ~570 established sockets to the agentex ClusterIP and served one inbound client apiece. Restarting only agentex-ui dropped backend Redis connections from ~1,094 to 279 without touching agentex, confirming the UI as the holder. Introduced in 0f07dc7 (#350).

Fix

Pass req.signal. App Router aborts it on client disconnect, which propagates through undici and destroys the upstream request — and makes the existing Istio route timeout an effective ceiling on upstream lifetime. A fixed timeout would be wrong here, since it would cut legitimate long-lived SSE streams.

The abort also rejects the in-flight fetch, so that case is answered with a 499 instead of propagating as a 500 — a departed client isn't an upstream failure. A genuine transport error still propagates exactly as before.

Verification

Built this branch, pointed AGENTEX_API_URL at a connection-counting SSE upstream, opened 5 streaming clients and killed them:

upstream live while connected 6s after clients killed
current main shape 5 5 ← leak
this branch 5 0

No ResponseAborted stack traces in the server log, server healthy afterward.

The main risk of adding a signal is truncating a response that is still streaming after the handler returns, so that was checked explicitly on a production build:

  • a 2 MB body streamed from upstream over ~2s arrived at exactly 2,048,011 bytes, 3/3 runs
  • a 666,669-byte streamed POST with duplex: 'half' echoed back intact

tsc --noEmit, eslint --max-warnings=0, and prettier --check are clean.

Follow-ups for the Agents team (not this PR)

Two agentex backend issues from the same thread remain open:

  1. cleanup_stream in the finally deletes a task-wide stream (streams_use_case.py:194). The topic is keyed by task id alone, so it is shared by every viewer. Worth landing with or right after this PR: today that finally almost never runs because disconnects don't propagate, and this change makes it run on every tab close. If something looks like a regression from this PR, it is that.
  2. SSE subscriptions have no terminal condition. A tab left open on a finished task still pins a pool connection indefinitely, polling XREAD every ~2.1s. The naive fix is unsafe: the client's retry loop (custom-subscribe-task-state.tsx, while (!signal?.aborted)) treats a clean server-side stream end as "reconnect immediately" — no backoff, no error-counter increment — so ending the stream server-side alone would produce a reconnect storm. Needs a coordinated client change.

Separately and unrelated to the leak: AGENTEX_API_URL falls back silently to http://localhost:5003, so a rename on either side yields a UI that can't reach the backend with no configuration error.

🤖 Generated with Claude Code

Greptile Summary

This PR fixes a connection-pool exhaustion bug where client disconnects (tab close, navigation, Istio timeout) left orphaned upstream SSE connections open indefinitely. The fix passes req.signal to the upstream fetch so that undici tears down the upstream request when the browser goes away, and wraps the fetch in a try/catch to return HTTP 499 for clean disconnects rather than propagating a 500.

  • Signal forwarding: signal: req.signal is added to the fetch options, making the Istio route timeout an effective ceiling on upstream lifetime and eliminating the Redis connection leak described in the incident.
  • Abort discrimination: The catch block uses identity comparison (error === req.signal.reason) rather than error.name === 'AbortError', correctly targeting Next.js's ResponseAborted abort reason while letting genuine transport errors propagate unchanged.

Confidence Score: 5/5

Safe to merge — a targeted, well-tested fix that eliminates the orphaned upstream connection leak with no changes to the happy path.

The change is small and surgical: one new try/catch block, one added fetch option. The abort discrimination via identity comparison correctly handles Next.js's ResponseAborted abort reason without relying on error names. The PR description documents explicit verification that streaming bodies are not truncated and that 0 upstream connections survive after all clients disconnect.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
agentex-ui/app/api/agentex/[...path]/route.ts Wraps upstream fetch in try/catch, adds req.signal to propagate client disconnects, and returns 499 on abort using identity comparison against req.signal.reason.

Reviews (2): Last reviewed commit: "fix(agentex-ui): match the abort by reje..." | Re-trigger Greptile

…nnects

The `/api/agentex` catch-all proxy streams SSE through unbuffered but passes
no signal to `fetch`, so when the browser goes away — closed tab, navigation,
or the 300s Istio route timeout — undici keeps the upstream request open. Each
orphan leaves an agentex handler blocked in a Redis XREAD, pinning one
connection from that pod's pool until it is exhausted and agentex can no
longer serve requests. There is no timeout on the call either, so an orphan
never expires.

Pass `req.signal`, which App Router aborts on client disconnect, and answer
the resulting fetch rejection with a 499 so a departed client is not reported
as an upstream failure. A fixed timeout would be wrong here — it would cut
legitimate long-lived SSE streams.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@erichwoo-scale
erichwoo-scale requested a review from a team as a code owner July 29, 2026 00:35
Comment thread agentex-ui/app/api/agentex/[...path]/route.ts
Addresses review feedback: `req.signal.aborted` alone can mask a genuine
transport failure that happens to coincide with a client disconnect, since the
flag is already set by the time the catch runs.

Match on `error === req.signal.reason` instead. `fetch` rejects with the
signal's abort reason itself, so identity separates the two cases without
consulting the flag at all. Matching on `error.name === 'AbortError'` would not
work here: Next's abort reason is a `ResponseAborted`, and the class name is
minified in a production build, so both name checks fail and every disconnect
would rethrow as a 500.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants